Conversation
This replaces the call that used a separate session with an rpc command.
WalkthroughThe pull request introduces a comprehensive refactoring of the remote configuration (RC) files installation process across multiple packages. The changes involve creating a new remote procedure call (RPC) mechanism for installing RC files, which replaces the previous direct SSH-based installation method. The modification spans several key components, including the addition of a new RPC command The refactoring removes direct SSH-based file installation logic and replaces it with a more flexible RPC-driven approach, which allows for better error handling, timeout management, and potentially more consistent cross-platform behavior. Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
cmd/wsh/cmd/wshcmd-rcfiles.go(2 hunks)frontend/app/store/wshclientapi.ts(1 hunks)pkg/remote/connutil.go(0 hunks)pkg/shellexec/shellexec.go(1 hunks)pkg/wshrpc/wshclient/wshclient.go(1 hunks)pkg/wshrpc/wshremote/wshremote.go(1 hunks)pkg/wshrpc/wshrpctypes.go(2 hunks)pkg/wshutil/wshutil.go(2 hunks)
💤 Files with no reviewable changes (1)
- pkg/remote/connutil.go
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Analyze (go)
- GitHub Check: Build for TestDriver.ai
🔇 Additional comments (7)
cmd/wsh/cmd/wshcmd-rcfiles.go (1)
Line range hint
8-24: LGTM! Clean refactoring of rcfiles command.The changes effectively centralize the path construction logic in the
wshutilpackage while maintaining proper error handling.🧰 Tools
🪛 golangci-lint (1.62.2)
22-22: printf: non-constant format string in call to github.com/wavetermdev/waveterm/cmd/wsh/cmd.WriteStderr
(govet)
pkg/wshrpc/wshremote/wshremote.go (1)
396-398: LGTM! Clean implementation of the RPC command.The implementation follows the established pattern and maintains proper error handling.
frontend/app/store/wshclientapi.ts (1)
250-254: LGTM! Clean implementation of the RPC client method.The implementation follows the established pattern and properly uses TypeScript's type system.
pkg/shellexec/shellexec.go (1)
308-311: LGTM! Error handling is preserved in the RPC migration.The change successfully migrates from direct client RC file installation to using the new RPC command while maintaining proper error handling and logging.
pkg/wshrpc/wshclient/wshclient.go (1)
304-308: LGTM! Implementation follows established patterns.The new RPC command is well-implemented, following the same patterns as other commands in the file:
- Consistent error handling
- Uses the common helper function
- Proper documentation
pkg/wshrpc/wshrpctypes.go (2)
77-77: LGTM! Command constant follows conventions.The new command constant follows the established naming pattern and value format used throughout the file.
185-185: LGTM! Interface method signature is consistent.The new method in
WshRpcInterfacemaintains consistency with other methods:
- Takes
context.Context- Returns
error- Follows naming convention
| func InstallRcFiles() error { | ||
| home := wavebase.GetHomeDir() | ||
| waveDir := filepath.Join(home, wavebase.RemoteWaveHomeDirName) | ||
| winBinDir := filepath.Join(waveDir, wavebase.RemoteWshBinDirName) | ||
| return shellutil.InitRcFiles(waveDir, winBinDir) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for home directory.
While the centralization of path construction logic is good, the function should handle the case where GetHomeDir() returns an empty string.
func InstallRcFiles() error {
home := wavebase.GetHomeDir()
+ if home == "" {
+ return fmt.Errorf("home directory not found")
+ }
waveDir := filepath.Join(home, wavebase.RemoteWaveHomeDirName)
winBinDir := filepath.Join(waveDir, wavebase.RemoteWshBinDirName)
return shellutil.InitRcFiles(waveDir, winBinDir)
}Committable suggestion skipped: line range outside the PR's diff.
Creates an RPC command for installing shell rcfiles instead of relying on a separate installation session.
1 participant
Creates an RPC command for installing shell rcfiles instead of relying on a separate installation session.